Skip to content

fix(runtime): carve liveness out of the identity step so a configuration fault cannot restart a pod - #16561

Merged
os-sales merged 1 commit into
mainfrom
claude/issue-15910-liveness-out-of-identity-step
Sep 7, 2026
Merged

fix(runtime): carve liveness out of the identity step so a configuration fault cannot restart a pod#16561
os-sales merged 1 commit into
mainfrom
claude/issue-15910-liveness-out-of-identity-step

Conversation

@os-sales

@os-sales os-sales commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Fixes #15910

A liveness probe that answers 503 for a configuration fault tells the orchestrator to restart a pod whose fault no restart can fix. This carries out the maintainer's ruling on #15910 (comment 5559821649, director seat, decision batch #57, 2026-09-06, maintainer reply verbatim 「同意」), option C:

Ruling. Carve liveness out of the identity step. /health (liveness) answers 200 whenever the process can serve HTTP, regardless of configuration faults; /ready (readiness) keeps returning 503 for the identity/configuration fault so traffic is withheld until the fault is fixed. A configuration fault must never restart a pod that cannot be fixed by restarting.

Option A (leave as shipped) and option B (make the posture read lazy for anonymous requests) are not taken; B was weighed and rejected in the ruling round for changing when a security-relevant value is read for every anonymous request.

The premise, measured on this branch — not inherited

PR #15909 is merged, so the behaviour is main's today. This branch measured it rather than quoting it: with the carve-out ablated out of dispatch() (one line, restored under a trap, byte-verified), the acceptance test reports

AssertionError: expected { health: 503, ready: 503 } to deeply equal { health: 200, ready: 503 }

That left-hand object is origin/main's answer to two uncredentialed probes on one kernel whose tenancy service is registered and fails to build.

/ready really does distinguish running from initializing — confirmed, with the nuance

Triage flagged this as reproduced-by-nobody and inherit-at-your-risk. Read at source (packages/runtime/src/http-dispatcher.ts, the /ready domain registration):

const state: string = typeof host?.getState === 'function' ? host.getState() : 'running';
if (state !== 'running') {
    return { handled: true, response: this.error('Service not ready', 503, { state }) };
}

with KernelState = 'idle' | 'initializing' | 'running' | 'stopping' | 'stopped' (packages/core/src/kernel-base.ts). So the claim holds, and the precise shape is worth stating: the branch is running versus everything else, not an initializing-specific arm — and the state that was read is echoed on the wire in the 503 body's details.state, so an operator reading it sees initializing told apart from stopping and stopped. The suite measures that echo (details.state is idle on an unstarted fixture kernel). /ready is therefore already the route meaning "this deployment is not serving correctly", and it is already wired.

What changed

A route may now declare liveness: true on its DomainRoute registry entry. HttpDispatcher.dispatch() runs such a route's handler directly — no resolveRequestScope, no ADR-0069 auth gate, no project-membership gate, so nothing that reads configuration or credentials is evaluated. /health declares it; its payload (status, timestamp, version, uptime) was already process-local, which is the ruling's first execution note satisfied by construction rather than by promise.

The identity step is the only thing /health loses. The two gates the carve-out also skips were already no-ops for this path, by measurement at source: enforceAuthGate opens with isAuthGateAllowlisted(cleanPath) and ALLOW_SUFFIXES in packages/core/src/security/auth-gate.ts names /health; enforceProjectMembership opens with a skipPaths list that names it too. So skipping them is observationally identical, and the whole behavioural delta of this PR is the one thing the ruling asked for.

Everything else is deliberately untouched:

  • /ready keeps the full identity step and its 503 body, so operator dashboards keep their signal (ruling's second execution note).
  • The fix(runtime): the tenancy posture seam tells "never registered" from "registered and failed" at the runtime door #15909 re-raise is not softened anywhere else: GET /data/task under the same fault still leaves dispatch() as a 503 SERVICE_UNAVAILABLE.
  • POST /health is not liveness (the route declares methods: ['GET'], and the carve-out inherits that).
  • The environment-scoped /environments/:id/health keeps today's behaviour end to end — the carve-out sits above the scoped-URL strip on purpose, and no orchestrator wires a scoped probe.

How the liveness route set is DERIVED, not listed

The card raised this to a binding constraint, citing this repo's own drifted hand-maintained lists (#15915, #16169, #15584). There is no new list:

resolveLiveness(path: string, method: string): DomainRoute | undefined {
    const route = this.resolve(path, method);
    return route?.liveness ? route : undefined;
}

The liveness set is a projection of the live route tableresolve() plus one field read — so it answers through the same matcher that picks the handler. It cannot name a route that is not registered, cannot miss one that is, and cannot disagree with the matcher about which route a path reaches (a non-liveness route registered earlier shadows here exactly as it shadows in resolve). A future liveness route becomes liveness by declaring it on the same object that makes the route exist, which is the only edit that cannot be forgotten.

The suite pins the derivation adversarially: it registers a new liveness route through the public registerDomainHandler seam and drives it through the same fault. A hard-coded '/health' test inside dispatch() would fail that leg alone while every other leg stayed green.

Docs (ruling's third execution note)

content/docs/deployment/self-hosting.mdx — the page that already carries the probe table and the reference Kubernetes manifest — gains the paragraph: which probe goes on livenessProbe, which on readinessProbe, never crossed, and what crossing them costs (CrashLoopBackOff in place of the fault). content/docs/deployment/cli.mdx's os serve section gains a short note and a link to it rather than a second copy of the field mapping, so the mapping stays in one place.

Tests

Run at e564d8be (git rev-parse --short HEAD of the commit under measurement).

Run Result
pnpm --filter @objectstack/runtime exec vitest run 239 files / 3373 tests passed
pnpm --filter @objectstack/runtime typecheck (tsc --noEmit + check:test-typecheck) exit 0
pnpm --filter @objectstack/http-conformance exec vitest run 5 files / 86 tests passed (the cross-adapter suite that locks dispatcher route semantics; needs @objectstack/runtime built, so it is measured after that build)
pnpm --filter @objectstack/hono exec vitest run 3 files / 82 tests passed
pnpm --filter @objectstack/cli exec vitest run --project unit 181 files / 2465 passed, 6 expected-fail (the unit tier; the integration tier spawns the CLI and is declared to CI, which runs both)
eslint . --no-inline-config --format json 6279 files, 0 errors, 0 warnings — the whole population eslint's own config selects, counted from the JSON report, not a narrowed subset
  • New: packages/runtime/src/http-dispatcher.liveness-carve-out.test.ts — 12 tests, all passing. The acceptance test the ruling names asserts the pair in one expression, against one forced fault:

    expect({ health: health.status, ready: ready.status }).toEqual({ health: 200, ready: 503 });

    Either half alone is non-discriminating: /health 200 alone is satisfied by deleting the re-raise outright, /ready 503 alone is satisfied by origin/main. Both probes are driven through the real mounted routes on a real ObjectKernel whose tenancy service factory throws, so the rejection under test is the service registry's own unbranded one.

  • The two 503s are told apart rather than assumed: under the fault /ready answers the identity step's declared-5xx envelope (message withheld, no details), while with a healthy tenancy on the same wiring it answers its own verdict (Service not ready, details.state). Without that control the readiness half would prove nothing.

  • Ablation, in the direction predicted: reverting the one carve-out lookup turns 4 of the 12 red (the pair, the payload, the dispatch-level leg, and the derived-set leg) and leaves 8 green — the 8 that describe unchanged behaviour. Mutation and restore were both proved on disk (occurrence counts before/after, git hash-object back to the HEAD blob, git diff HEAD empty), under a trap ... EXIT INT TERM. No dist is involved: the suite imports ./http-dispatcher.js relatively, which resolves to source — demonstrated by the fact that the new resolveLiveness leg passes on a tree where @objectstack/runtime was never rebuilt, while the cross-package http-conformance suite could not resolve the package at all until it was.

Gates

Derived from the tree at runtime, no paths passed: node scripts/pm/dispatch-gates.mjs --commands (repo asserted with --repo objectstack-ai/objectstack), each command run byte-for-byte as printed and recorded as it ran. Reconciled against the tool's own count, not against arithmetic over my list:

✓ dispatch-gates --ran: 94 derived famil(ies) accounted for — 94 run, 0 NOT-MEASURED.

93 of the 94 pass. The one that does not is PREREQUISITE NOT MET (exit 3) — nothing measured, neither a pass nor a finding, and it says so itself: check:dual-build-cjs-loads reads built output for 38 packages that have no dist in this worktree. CI builds fresh and measures it.

check:type-check-debt first reported the same exit 3 here, for a different reason — its --re-measure leg exhausted a 4 GB heap on a whole-tree tsc and refused to record a 0. Re-run with --max-old-space-size=8192 it passes: OK — 76/80 workspace packages type-checked, 4 in the DEBT ledger, with tsc itself still under the gate's own pinned 6144 MB CI-shaped ceiling. The first reading was an environment limit, not a ratchet movement.

The separate artifact-roster block (36 families, 46 command spellings) was run on its own, as it sits beside the derived total rather than inside it: 43 pass, and the three that do not are wiring or prerequisite, not verdicts. check:partof-closing-keyword prints NOT WIRED with no PR context and was then run wired — this body plus this PR's commit list — and passes. check:single-claim-paths needs a GitHub token to list the PR's files; retried against this PR number it gets GitHub API 401, so it stays NOT MEASURED here and CI measures it. check:react-declaration-parity has no objectui sdui.manifest.json to compare against, which is by design: it is an on-demand gate triggered by an .objectui-sha bump this PR does not make.

Full per-command results are in the report comment on #15910.

Clause ②

no, re-derived from the delivered diff (the claim's declaration was no; this is the re-derivation the dispatch required, not an inheritance):

  • Path limb: zero files under packages/spec/src/**. No *.zod.ts contract schema, no ERROR_CODE_LEDGER / StandardErrorCode entry, no new error code.
  • Wire limb: both bodies involved are pre-existing declared shapes and are byte-unchanged. What moved is which declared body one route returns under a fault — a runtime behaviour change, which the criteria place on the explicit negative boundary ("a runtime permission/security behaviour change is not clause ②").
  • The additive TypeScript surface (DomainRoute.liveness, DomainHandlerRegistry.resolveLiveness) is API-surface widening on @objectstack/runtime, paid for by the minor changeset — not a published contract schema.

No needs:contract-review label is applied.

Base

Branched at f48f3f1b2 and not merged with origin/main, deliberately and declared rather than skipped: everything above was measured on that base, and the commits that landed since (through ce8caba91) touch 17 files with zero overlap with this PR's 7, and none of them touches the dispatcher's identity step, the domain registry, or probe routing. AGENTS.md scopes the pre-PR re-run to incoming commits that touch the same packages or the same behaviour; these do not. The PR's own CI builds the merge ref, and the merge queue re-verifies on the rebuilt generation, so the landing seat's merge is the right place for it — merging here would only invalidate the readings above against a main that moves every few minutes on this container.

验收备注

Observed while reading, not filed and not fixed here — none is a reproducible defect, a violation of declared contract text, or a metadata-authoring trap:

  • Two pre-existing hand-written path lists already name /health: ALLOW_SUFFIXES in packages/core/src/security/auth-gate.ts and skipPaths in HttpDispatcher.enforceProjectMembership. Both cover more than liveness (/auth, /cloud, /ready, /discovery) and are read from more than one door, so neither is subsumed by the derived liveness set and neither was touched. Worth knowing that the liveness carve-out now precedes both for /health.
  • packages/runtime/src/route-ledger.ts carries a third place that calls GET /health a liveness probe, in prose on its ledger row. It is ratcheted by route-ledger.conformance.test.ts against the dispatcher's domains, so it is not an unwatched list; left as is.

…lt cannot restart a pod

`HttpDispatcher.dispatch()` resolved a per-request identity before any route
handler ran, and that step reads the tenancy posture for every request —
credentialed or not. A `tenancy` service that is registered and fails to build
is re-raised as 503 rather than absorbed into "there is no posture", so an
uncredentialed liveness probe was answered 503 for the length of the outage: a
liveness 503 means "restart me", the service fails to build again on the new
pod, and the restart loop hides the fault the 503 exists to make loud.

A route may now declare `liveness: true` on its registry entry. `dispatch()`
runs such a route's handler directly — no identity resolution, no gate, nothing
that reads configuration or credentials. `/health` declares it; its payload was
already process-local. `/ready` is untouched and keeps the full identity step
and its 503 body, so traffic is still withheld until the fault is fixed.

Which routes count as liveness is DERIVED from the dispatcher's own route
table: `DomainHandlerRegistry.resolveLiveness()` is `resolve()` plus one field
read, so it answers through the same matcher that picks the handler and cannot
drift from the routes that exist. There is no second list of paths.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01YFY46JydE1gMxQG1TqBcMZ
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation labels Sep 7, 2026
@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

5 anchor(s) derived from 1 changed package(s); no hand-written page names any of them, so this run has nothing to listnot a clean bill of health. This check sees only pages that NAME a derived anchor: one that documents this change in prose, or enumerates it in an authoring dialect, names none and stays invisible to it on every run.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ce8caba91403c8f160cb7764c63b08371a13db99packageMentionDocs.

Which tree this was computed on

This run read content/docs from 5a497465412d73d76e11ebe7ebad5471991d7f3b — the merge of head e564d8bebf1e4f616be052fc5afb078541d70443 into base ce8caba91403c8f160cb7764c63b08371a13db99, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 5a497465412d73d76e11ebe7ebad5471991d7f3b && git checkout 5a497465412d73d76e11ebe7ebad5471991d7f3b
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ce8caba91403c8f160cb7764c63b08371a13db99 e564d8bebf1e4f616be052fc5afb078541d70443 && git checkout -B drift-repro ce8caba91403c8f160cb7764c63b08371a13db99 && git merge --no-ff e564d8bebf1e4f616be052fc5afb078541d70443

node scripts/docs-audit/affected-docs.mjs --json ce8caba91403c8f160cb7764c63b08371a13db99

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

@os-sales
os-sales marked this pull request as ready for review September 7, 2026 10:10
@os-sales
os-sales enabled auto-merge September 7, 2026 10:11
@os-sales
os-sales added this pull request to the merge queue Sep 7, 2026
Merged via the queue into main with commit 3e560da Sep 7, 2026
39 checks passed
@os-sales
os-sales deleted the claude/issue-15910-liveness-out-of-identity-step branch September 7, 2026 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

2 participants